Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

feat: added tools name format validation accordingly #SEP-986 #764

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ashakirin wants to merge 1 commit into modelcontextprotocol:main
base: main
Choose a base branch
Loading
from ashakirin:feature/sep-986-validate-tool-name-format

Conversation

@ashakirin
Copy link
Contributor

@ashakirin ashakirin commented Jan 29, 2026

Added tools name format validation accordingly #SEP-986
Note: tool duplications are already checked (in McpAsyncServer.java and in McpStatelessAsyncServer.java)

@Kehrlann Kehrlann self-requested a review January 30, 2026 08:58
Copy link
Contributor

Thanks for your contribution @ashakirin ! A few comments.

Optional validation

The spec specifically says that tools SHOULD follow the naming conventions (Server > Tools > Tool names). They MAY not follow these, and so we should allow invalid names (although not by default).
We could consider a per-server validator, that you plug in the SyncSpecification and AsyncSpecification. Thoughts?

Client-side validation

I like the approach on the client side, validate but don't fail. The spec itself is super vague, "client can validate" which has no RFC meaning. Maybe we shouldn't even validate on the client side.

If we chose to validate, in the current implementation the log is very verbose if you're writing a client and the server (that you don't control) has an invalid tool name. Every time you make a "call tool request" you get a WARN-level log. I think that's too much, and the level is probably too high. In that case, keep the logs for the "list tool" request but not "call tool".

Copy link
Contributor Author

@Kehrlann: Thanks for detail review, Daniel!
My thoughts:

Optional validation

The spec specifically says that tools SHOULD follow the naming conventions (Server > Tools > Tool names). They MAY not follow these, and so we should allow invalid names (although not by default). We could consider a per-server validator, that you plug in the SyncSpecification and AsyncSpecification. Thoughts?

My initial interpretation of SHOULD was that the SDK may choose whether to support validation at all, not that validation of server-side tool names should be optional. That seems to be incorrect.

Proposal: using different validation options for different MCP servers registered in the same application is probably not a very typical use case. I would therefore propose a global setting to disable validation, with an option to override it per server in SyncSpecification, AsyncSpecification, StatelessAsyncSpecification, and StatelessSyncSpecification. WDYT?
This keeps the default behavior simple while still allowing flexibility when integrating with non-compliant servers. What do you think?

Client-side validation

I like the approach on the client side, validate but don't fail. The spec itself is super vague, "client can validate" which has no RFC meaning. Maybe we shouldn't even validate on the client side.

If we chose to validate, in the current implementation the log is very verbose if you're writing a client and the server (that you don't control) has an invalid tool name. Every time you make a "call tool request" you get a WARN-level log. I think that's too much, and the level is probably too high. In that case, keep the logs for the "list tool" request but not "call tool".

My idea was to validate tool names via deserialization on both the client and server sides: on the client during listTools, and on the server during tool/call.
I agree that the current implementation in compact constructors is too verbose, because:

  • the client constructs the CallToolRequest itself
  • the compact listTools constructor is invoked twice due calling of pagination / cursors constructor

Therefore I am going to move validation into McpAsyncClient.listToolsInternal().
Do you think it makes sense to validate tool names on the server side during tool/call? Basically a client can call server with non-existent and invalid tool name, but that will result in a "tool not found" error. And existing invalid server-side tool names will be already validated during tool registration on the server side - so additional validation seems doesn't add value.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Kehrlann Kehrlann Awaiting requested review from Kehrlann

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /